Skip to content

[full-ci] [OCISDEV-460] fix: move the home space create cache to the proxy#12128

Merged
2403905 merged 1 commit intomasterfrom
fix/OCISDEV-460
Apr 23, 2026
Merged

[full-ci] [OCISDEV-460] fix: move the home space create cache to the proxy#12128
2403905 merged 1 commit intomasterfrom
fix/OCISDEV-460

Conversation

@2403905
Copy link
Copy Markdown
Contributor

@2403905 2403905 commented Mar 18, 2026

Description

move the home space create cache to the proxy

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@paul43210
Copy link
Copy Markdown
Contributor

@2403905 This is a much cleaner approach — removing the reva-side cache entirely and keeping it simple in the proxy makes a lot of sense. Happy to help with this PR in any way.

A couple of observations from working on the original #12115:

  1. Concurrent request deduplication: The original issue (The Proxy service middleware creating a personal space multiple times #12068) reported 11 simultaneous CreateHome calls on first login. The ttlcache.Has() check will prevent repeated calls after the first one succeeds, but during that initial window (before the first CreateHome returns), concurrent requests will still all fire independently. A singleflight.Group wrapping the CreateHome call would collapse those into a single gRPC call. Happy to add this if you'd like.

  2. Tests: I had unit tests from fix(proxy): [OCISDEV-460] deduplicate CreateHome calls in middleware #12115 that could be adapted for this implementation if that would be useful.

Let me know if there's anything I can pick up here.

@2403905 2403905 force-pushed the fix/OCISDEV-460 branch 3 times, most recently from f309679 to e105384 Compare March 20, 2026 12:39
@2403905 2403905 marked this pull request as ready for review March 20, 2026 12:39
@2403905 2403905 requested review from jvillafanez and kobergj March 20, 2026 12:52
@2403905
Copy link
Copy Markdown
Contributor Author

2403905 commented Mar 20, 2026

@paul43210 Thank you for the reply. I think the tests are redundant in this case.

@2403905 2403905 force-pushed the fix/OCISDEV-460 branch 2 times, most recently from 76b707e to 080cef6 Compare March 20, 2026 14:58
@2403905 2403905 enabled auto-merge March 20, 2026 15:14
@sonarqubecloud
Copy link
Copy Markdown

Comment thread services/proxy/pkg/middleware/create_home.go Outdated
@jvillafanez
Copy link
Copy Markdown
Member

I don't think we can have a good solution to this. Users can be created outside of oCIS (Keycloak, for example), so we can't create their home when the user is created.
The cause of the multiple requests is because the CreateHome happens in the middleware, without any real control. This means that any request going through the proxy will trigger the request (unless any previous middleware stops the request).

Screenshot from 2026-03-25 15-21-47 Screenshot from 2026-03-25 15-22-04

(There is only one GatewayAPI/Authenticate call. It's the same request from the screenshot above it.)


The approach looks fine to me. Just a couple of notes:

  • The ttlcache library seems too much for what we need. Not sure if we want to add the library. The ocis-pkg/sync/cache.go should provide something similar. We might need to double-check details.
  • I'm not fond of spawning a goroutine that will effectively be running forever. I'm not sure if it will interfere with the normal shutdown procedure because, at the moment, we aren't stopping the goroutine. The ocis-pkg/sync/cache.go doesn't need an additional goroutine, and it keeps the memory usage under control by limiting the capacity of the cache.

@2403905
Copy link
Copy Markdown
Contributor Author

2403905 commented Mar 26, 2026

I don't think we can have a good solution to this. Users can be created outside of oCIS (Keycloak, for example), so we can't create their home when the user is created. The cause of the multiple requests is because the CreateHome happens in the middleware, without any real control. This means that any request going through the proxy will trigger the request (unless any previous middleware stops the request).

  • The ttlcache library seems too much for what we need. Not sure if we want to add the library. The ocis-pkg/sync/cache.go should provide something similar. We might need to double-check details.
  • I'm not fond of spawning a goroutine that will effectively be running forever. I'm not sure if it will interfere with the normal shutdown procedure because, at the moment, we aren't stopping the goroutine. The ocis-pkg/sync/cache.go doesn't need an additional goroutine, and it keeps the memory usage under control by limiting the capacity of the cache.

I agree it is not the best solution. The goal I reached in this pull request is to push the part of the changes from the #12108

I chose the github.com/jellydator/ttlcache/v3 because this package is already used in a proxy.

@2403905 2403905 requested a review from jvillafanez March 26, 2026 12:34
@2403905 2403905 disabled auto-merge March 26, 2026 13:18
@2403905 2403905 enabled auto-merge April 23, 2026 18:14
@2403905 2403905 removed the request for review from kobergj April 23, 2026 20:50
@2403905 2403905 merged commit dacca91 into master Apr 23, 2026
54 checks passed
@2403905 2403905 deleted the fix/OCISDEV-460 branch April 23, 2026 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The Proxy service middleware creating a personal space multiple times

3 participants